Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: redial node_addresses at an interval on connection close #529

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

QuinnWilton
Copy link
Contributor

@QuinnWilton QuinnWilton commented Jan 23, 2024

This resolves #400.

@QuinnWilton QuinnWilton requested a review from a team as a code owner January 23, 2024 22:39
Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (0d778d3) 67.50% compared to head (d83af5f) 77.34%.
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #529      +/-   ##
==========================================
+ Coverage   67.50%   77.34%   +9.83%     
==========================================
  Files          83       83              
  Lines        9635     9537      -98     
==========================================
+ Hits         6504     7376     +872     
+ Misses       3131     2161     -970     
Files Coverage Δ
homestar-runtime/src/event_handler.rs 100.00% <100.00%> (+4.22%) ⬆️
homestar-runtime/src/event_handler/cache.rs 77.50% <100.00%> (+55.00%) ⬆️
homestar-runtime/src/event_handler/event.rs 62.66% <100.00%> (+54.73%) ⬆️
homestar-runtime/src/settings/libp2p_config.rs 100.00% <100.00%> (+4.61%) ⬆️
homestar-runtime/src/event_handler/swarm_event.rs 31.38% <83.05%> (+27.23%) ⬆️

... and 55 files with indirect coverage changes

@QuinnWilton
Copy link
Contributor Author

QuinnWilton commented Jan 24, 2024

Briefly chatted with Brian and we came up with an idea for writing an integration test for this, so I'll give that a try still, + also handle retrying when a redial fails to reestablish the connection.

I also dug into the libp2p-swarm code, and we should be good on invoking dial inside of handle_info: it looks like that call only creates the futures for each dial attempt, and then inserts them into a pool for processing elsewhere.

@QuinnWilton QuinnWilton force-pushed the quinn/redial-nodes branch 5 times, most recently from 1f44a7a to 31de9e4 Compare January 25, 2024 18:56
Copy link
Contributor

@bgins bgins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! ✨ Thanks for taking this one on. Left a comment for one small change.

homestar-runtime/src/event_handler/swarm_event.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@zeeshanlakhani zeeshanlakhani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor thing/warning thing or two on warnings/tests. Otherwise 👍🏽, great work @QuinnWilton.

homestar-runtime/tests/network/notification.rs Outdated Show resolved Hide resolved
@zeeshanlakhani zeeshanlakhani merged commit d14abcb into main Jan 29, 2024
31 checks passed
@zeeshanlakhani zeeshanlakhani deleted the quinn/redial-nodes branch January 29, 2024 20:03
@release-plz-ipvm-wg release-plz-ipvm-wg bot mentioned this pull request Jan 29, 2024
zeeshanlakhani pushed a commit that referenced this pull request Feb 21, 2024
## 🤖 New release
* `homestar-runtime`: 0.1.1 -> 0.2.0 (⚠️ API breaking changes)
* `homestar-invocation`: 0.1.1 -> 0.2.0 (✓ API compatible changes)
* `homestar-wasm`: 0.1.1 -> 0.2.0 (✓ API compatible changes)
* `homestar-workflow`: 0.1.1 -> 0.2.0 (✓ API compatible changes)

### ⚠️ `homestar-runtime` breaking changes

```
--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.28.0/src/lints/enum_variant_added.ron

Failed in:
  variant Command:Node in /tmp/.tmp7PLiiL/homestar/homestar-runtime/src/cli.rs:149
  variant Command:Info in /tmp/.tmp7PLiiL/homestar/homestar-runtime/src/cli.rs:155
```

<details><summary><i><b>Changelog</b></i></summary><p>

## `homestar-runtime`
<blockquote>

##
[0.2.0](homestar-runtime-v0.1.1...homestar-runtime-v0.2.0)
- 2024-02-20

### Added
- Add OpenRPC API docs and associated JSON Schemas
([#534](#534))
- redial `node_addresses` at an interval on connection close
([#529](#529))

### Fixed
- add handling of dns multiaddrs + bootstrapping + CLI / Conn changes
([#547](#547))

### Other
- deps + flake cleanup
([#581](#581))
- Allow dead code default timeout
([#577](#577))
- Update homestar-functions to use cargo component
([#576](#576))
- fix transport order for wss possibility
([#563](#563))
- small comment, sorry
([#561](#561))
- move away from deadlines dealing w/ the runner and wasi-preview 2
wasmtime ([#560](#560))
- docker updates with info command and rpc host update
([#558](#558))
- just test conn ([#544](#544))
- handle this evil workflow_info test
([#543](#543))
- remove unnecessary deps and add tooling for those checks
([#541](#541))
- [chore(cargo)](deps): bump puffin from 0.18.1 to 0.19.0
([#537](#537))
- updates/flaky kills on ci
([#540](#540))
- release docs and cp readmes
([#530](#530))
- port selection and test config generation macro
([#528](#528))
- [chore(cargo)](deps): bump serde_with from 3.4.0 to 3.5.0
([#524](#524))
- [chore(cargo)](deps): bump moka from 0.12.3 to 0.12.4
([#525](#525))
</blockquote>

## `homestar-invocation`
<blockquote>

##
[0.2.0](homestar-invocation-v0.1.1...homestar-invocation-v0.2.0)
- 2024-02-20

### Added
- Add OpenRPC API docs and associated JSON Schemas
([#534](#534))

### Other
- deps + flake cleanup
([#581](#581))
- Update homestar-functions to use cargo component
([#576](#576))
- move away from deadlines dealing w/ the runner and wasi-preview 2
wasmtime ([#560](#560))
- remove unnecessary deps and add tooling for those checks
([#541](#541))
- release docs and cp readmes
([#530](#530))
</blockquote>

## `homestar-wasm`
<blockquote>

##
[0.2.0](homestar-wasm-v0.1.1...homestar-wasm-v0.2.0)
- 2024-02-20

### Other
- deps + flake cleanup
([#581](#581))
- Update homestar-functions to use cargo component
([#576](#576))
- move away from deadlines dealing w/ the runner and wasi-preview 2
wasmtime ([#560](#560))
- remove unnecessary deps and add tooling for those checks
([#541](#541))
- release docs and cp readmes
([#530](#530))
</blockquote>

## `homestar-workflow`
<blockquote>

##
[0.2.0](homestar-workflow-v0.1.1...homestar-workflow-v0.2.0)
- 2024-02-20

### Added
- Add OpenRPC API docs and associated JSON Schemas
([#534](#534))

### Other
- deps + flake cleanup
([#581](#581))
- remove unnecessary deps and add tooling for those checks
([#541](#541))
- release docs and cp readmes
([#530](#530))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Signed-off-by: release-plz-ipvm-wg[bot] <144082651+release-plz-ipvm-wg[bot]@users.noreply.github.com>
Co-authored-by: release-plz-ipvm-wg[bot] <144082651+release-plz-ipvm-wg[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Networking: use event_handler cache to (re-)dial disconnected or connection-error node addresses
3 participants